Improve handling of key mapping #325
Conversation
This is called in the onOpen function two lines later. Besides, we don't want to map the keys until after we check if the current filetype is active.
|
This is an interesting feature, it goes against my personal workflow but I'm curious to try it out. Are there specific keys that you want to be able to use their default behavior when the server isn't running? I typically rely on the error that a server isn't running to recognize what is happening, I worry that if a server dies (and I don't notice the error about it) and I try a keybind it will be more confusing to have it unmapped... I'll give this a try and see how it feels though. |
|
The main reason I find this useful is in two scenarios:
If the server isn't running, then I want |
autoload/lsc/server.vim
Outdated
| @@ -87,6 +87,7 @@ function! s:Kill(server, status, OnExit) abort | |||
| call a:server._channel.notify('exit', v:null) | |||
| endif | |||
| if a:OnExit != v:null | call a:OnExit() | endif | |||
| call lsc#config#unmapKeys() | |||
There was a problem hiding this comment.
Looks like this will unmap keys for whichever buffer is current when the server exists - there isn't a guarantee that the current buffer is one handled by the server which exited, and we'll missed unmapping keys on background buffers as well.
I think we'll need to loop over buffers for each filetype and unmap in each of them which makes things a little trickier.
There was a problem hiding this comment.
You're right, this is tricky because as far as I know Vim doesn't provide a method for (un)mapping buffer-local keys for buffers other than the current one. Which means to unmap the buffer-local keys in all buffers associated with the language server, we would either need to:
-
Iterate through each buffer and unmap the keys (probably slow and requires switching buffers which has all other kinds of pitfalls)
-
Hook into the BufEnter autocmd and check the status of the server associated with that buffer's filetype: if the status is disabled then unmap the keys. This essentially unmaps "on demand".
Unless there is something I'm overlooking then it seems to me that option 2 is the clear winner. I will update the PR with these changes for your review.
This allows keys to be unmapped much easier by simply iterating through the list of keys that have been mapped.
When a language server exits for any reason, we want to unmap the vim-lsc mappings in all of its corresponding buffers (so that we don't have "dead" mappings lying around). To do this, we can either 1. Iterate through all open buffers and unmap buffer-local mappings whenever the server quits 2. Unmap buffer-local mappings on-demand when the buffer is opened, if the server is no longer running Option 2 is the better choice (and is what this commit implements) as it's faster and leaner (iterating through all open buffers and removing mappings can be *very* slow if there are a lot of buffers open).
|
Hi Nate, I'm wondering if you've had another chance to review this? I ran into this issue again recently while working in the Linux kernel source code. vim-lsc started up with the |
Only map keys when server is successfully initialized or is already running. This prevents the scenario where LSC still maps keys even if the server fails to initialize (in which case the key mappings are useless).
Unmap keys when the server shuts down. Similar to point 1, if the user runs
:LSClientDisablethen the LSC mappings should be removed.